Add react-native-web support #3958

zoriya · 2024-06-30T13:32:21Z

Summary

This adds web support for RNV. It does not include ads, drm, hls or any fancy player options not supported by an HTML5 player. I think HLS.js would be good to have included by default but if we do, it should be in a FU PR.

For now this PR is in a very draft state, I have not yet tested nor updated the documentation. I'll do it in the following hours/days (I'm currently working from my hospital bed, so do not know how much work I'll do soon).

I made this draft mostly to get preliminary feedback on how I approached this.

Closes #2719

Missing steps

Test (see testing plan)
Write documentation/readme

Test plan

integrate or create a web example
~~use this in Kyoo, I took the base implementation from there and the projects needs HLS and custom handling for subtitles for example.~~ Will do after this is merged and we try to add shaka/video.js or something like that.

shell.nix

zoriya · 2024-06-30T13:33:36Z

src/Video.web.tsx

+    return (
+      <video
+        ref={nativeRef}
+        src={source?.uri as string | undefined}


Non URL sources are not supported, I guess there's no way arround it.

no, but may need a comment in the the doc (even it doesn't really make sense on web I think ... )

Added: https://github.com/TheWidlarzGroup/react-native-video/pull/3958/files#diff-e2131d0631cdc1ddf0ba36827a27b86adf8e5b8ed162a280a53c6219f4df5a5fR643

zoriya · 2024-06-30T13:34:04Z

src/Video.web.tsx

+          onLoad?.({
+            currentTime: nativeRef.current.currentTime,
+            duration: nativeRef.current.duration,
+            videoTracks: [],


HTML5 does not give us tracks info (nor a way to select the current video/audio.

add note in doc :)

Already done: https://github.com/TheWidlarzGroup/react-native-video/pull/3958/files#diff-ea60cfe144c95a06a3ecdc85bf16b43620ba5711bfe2d01ea18d69da24050445R223

src/Video.web.tsx

zoriya · 2024-06-30T13:36:07Z

src/Video.web.tsx

+          onVolumeChange?.({volume: nativeRef.current.volume});
+        }}
+        onEnded={onEnd}
+        style={{position: 'absolute', inset: 0, objectFit: 'contain'}}


I can't use the StyleSheet api since this is a raw html component. Should I simply add a comment to disable the eslint warning here?

Do you have the issue if you move the style in a const variable ?
I think it would be better

src/Video.web.tsx

zoriya · 2024-06-30T13:39:18Z

src/types/video-ref.ts

+  setVolume: (volume: number) => void;
+  getCurrentPosition: () => Promise<number>;
+  setFullScreen: (fullScreen: boolean) => void;
+  nativeHtmlRef?: RefObject<HTMLVideoElement>; // web only


This would be needed for a user to add custom HTML5 libs (like hls.js, video.js or better subtitles handling).

I did not document this, if we go into integrating vifdeo.js/shaka/something this would not be needed anymore and then removed.

@zoriya Is it a good idea to expose nativeHtmlRef to the UI ? do it bring some additionnal feature .
maybe you can rename it to nativeHtmlVideoRef it will allow to create new values for shaka / video.js, ...

Not sure where to add documentation for this, maybe in the method page? Since it's the page that talk about the video ref?

I added it on the method page

zoriya · 2024-07-01T04:25:31Z

I tested that everything works with the basic example app using one of the .mp4 videos available without headers (since both HLS/Dash and headers are not available with a HTML5 basic video player).

I'm thinking that RNV should have more features than the default browsers allow, and integrating HLS/Dash/Ads should not be left for the user.

On my project, I'm using HLS.js, srt-webvtt and jassub to have hls/srt/ass support. Maybe we could find an extensible and not too bloaty library to use as a video backend on the web. I'm not familiar with the state of those libs right now but I have heard of video.js and shaka-player.

What do you think about integrating one of those libraries @freeboub @KrzysztofMoch?

KrzysztofMoch · 2024-07-01T12:13:11Z

I'm thinking that RNV should have more features than the default browsers allow, and integrating HLS/Dash/Ads should not be left for the user.

@zoriya I agree with you - but let's do it in next (separate) PR

I will try to review this PR this week!

examples/basic/package.json

src/specs/VideoNativeComponent.ts

KrzysztofMoch

I have tested this in our basic example (with expo web) and it's working very well!
For now are working only mp4 files but support for other let's leave for later. I will look into code now.
Overall grate job @zoriya 🔥

zoriya · 2024-07-08T11:22:55Z

Thanks, I'll correct review comments & rebase during the night!

src/Video.web.tsx

KrzysztofMoch · 2024-07-10T09:15:31Z

src/Video.web.tsx

+        // making the video fullscreen does not work with some subtitles polyfils
+        // so I decided to not include it.


You mean that some subtitles polyfils are not working when video is in fullscreen ? Users can open fullscreen from UI so I think we can implement those and add warning in docs WDYT?

Yes, some subtitles polyfill use a canvas or other HTML elements to draw on top of the video, and those do not work if we simply full-screen the video.

I was thinking of implementing the full-screen feature after adding the polyfill, since it's possible to fullscreen a container of both the video and the polyfill.

I understand, but can we for now implement "built-in" fullscreen? We can later edit it to support subtitles polyfills

Kay, i'll add that

Yes, that's what I used. Controls can't be shown using this tho.

CleanShot.2024-07-10.at.15.53.00.mp4

I have checked it and it's working

Built-in (HTML5 defaults) controls will show but not custom ones

I see three ways of fixing this:

have a props for a ref of the item that needs to be in fullscreen (could be a ref to document.body)

have controls as child of the Video element

add an html id/class player-controls and we could find the closest parent with this class to fullscreen

The second one is the most "react-like" but it kinda breaks expectation of having a simple Video element + it's a breaking.

The other two feels more like escape hatch.

What does the fullscreen behavior does on android/ios? I must say I never used it and always used fullscreen on the whole route via the router.

Fullscreen on iOS/Android also doesn't allow custom controls so it's not a problem (maybe in future we will add it)

freeboub · 2024-07-15T21:13:29Z

@zoriya I still 2 documentation comments to handle. + conflicts to fix . And it is good on my side.
@KrzysztofMoch not sure about timeline to merge this PR ?!

zoriya · 2024-07-16T05:34:17Z

src/types/index.ts

@@ -7,4 +7,4 @@ export {default as ResizeMode} from './ResizeMode';
 export {default as TextTrackType} from './TextTrackType';
 export {default as ViewType} from './ViewType';
 export * from './video';
-export * from '../specs/VideoNativeComponent';


I think this was only used for event types so I removed it and added a

export type * from '../spec/VideoNativeComponent';

in the events.ts file. I assumed the default export was not used, if it's used I can restore it like before and add a file that export a dummy object for the web.

zoriya · 2024-07-16T05:35:27Z

I rebased & added all new properties. Everything should be documented, I missed the nativeVideoHtmlElement and added it, other doc comments should be fixed.

Ramon-Balaguer · 2024-07-22T15:29:43Z

@zoriya How is this feature?

zoriya · 2024-07-22T15:31:23Z

what does this mean? The PR just needs to be reviewed again/approved if that was the question

mrsantran · 2024-07-25T09:39:53Z

merge this PR please :)

src/Video.web.tsx

zoriya · 2024-11-06T20:18:55Z

What's the expected behavior when using setSource and the source prop is defined/changes?
Should using setSource disable the source prop? Should 'setSourceonly work ifsource` is undefined?

zoriya · 2024-11-06T20:27:48Z

Except the setSource method that doesn't handle metadata nor interactions with the source prop (see my previous comment), this should be ready.

Once this get merged, I'll make follow-up PR for hls & subtitles while I transition kyoo to use this for the web.

KrzysztofMoch · 2024-11-13T09:07:27Z

What's the expected behavior when using setSource and the source prop is defined/changes?
Should using setSource disable the source prop? Should 'setSourceonly work ifsource` is undefined?

So this is good question 🤔 on mobile we do re-init on source change. setSource is overriding source prop but if source prop would be updated it will override source from setSource

So on web I guess with should do something like this ?

const [source, setSource] = React.useState(props.source)

useEffect(() => {
  setSource(props.source)
}, [props.source])

// and add method to ref

WDYT @zoriya ?

zoriya · 2024-11-13T09:24:17Z

This is assuming that source is stable (aka wrapped in a useMemo). Without this, each re-render (be it another prop change or a non-relevant setState) would reset the setSource.

I am fine with this behavior, if we decide to go with this. I'll also add it in the documentation.

KrzysztofMoch · 2024-11-13T09:27:13Z

Yeah you're right! but do we have any alternative? (idk if there is, just thinking loudly)

KrzysztofMoch · 2024-11-13T09:35:18Z

Maybe we could use React.memo with custom arePropsEqual function?

zoriya · 2024-11-13T09:37:54Z

No, I don't think there's a good alternative available.
We could hold a ref to the old source and deep compare the objects, but that felt so wrong that I decided to ask what was the expected behavior.

Something like:

const [src, setSource] = useState(source)
const oldSource = useMemo(source);
useEffect(() => {
	if (deepEqual(oldSource.current, source)) 
		return;
	setSource(source);
	oldSource.current = source;
}, [source]);

// add setSource to ref

With deepEqual being a function from something like here.

KrzysztofMoch · 2024-11-13T09:43:55Z

Yeah this looks kinda evil 😅 But I think this will work best, also I would rename oldSource to currentSource (and here you are using useMemo but I guess you wanted to use useRef) - we can go with it

zoriya · 2024-11-13T10:07:08Z

oh yeah i meant useRef. I'll push that today then x)

KrzysztofMoch

Thank you very much!
@moskalakamil can you look at this and test it? If you will be fine you can merge

moskalakamil · 2024-11-13T13:17:54Z

Okay, I will check it later today. Thank you so much, @zoriya. Amazing work!

moskalakamil · 2024-11-13T14:48:23Z

@zoriya We need to update constants/general.ts in the bare example as well, because the Expo example is overridden in the src folder every time we install node modules (I mean this lines:

metadata: {
      title: 'Custom Title',
      subtitle: 'Custom Subtitle',
      artist: 'Custom Artist',
      description: 'Custom Description',
      imageUri:
        'https://pbs.twimg.com/profile_images/1498641868397191170/6qW2XkuI_400x400.png',
}

moskalakamil · 2024-11-13T20:17:32Z

LGTM! Amazing work @zoriya 🔥

freeboub · 2024-11-17T22:01:08Z

@zoriya I misse to thank you for this PR, so Thank you very much !

colonelpanic8 · 2024-12-03T03:20:42Z

@zoriya I vaguely remember that you mentioned you were planning to work on hls support.

I actually have a version of this working, but its kind of sloppy, but I'm wondering if you had made any inroads with that yourself. Might be helpful for us to coordinate.

zoriya · 2024-12-03T08:45:02Z

I wanted to wait for #4255 to get merged before creating a new branch to prevent huge conflicts. I do already have hls support on kyoo which was the implementation I based this PR on (here).

KrzysztofMoch · 2024-12-03T08:47:43Z

I need to fix expo example to merge it (but no idea how to do it for now)

colonelpanic8 · 2024-12-03T18:21:06Z

@zoriya okay interesting. doesn't look like that applies super cleanly on what is there though. What was the reasoning behind using hls.js instead of videojs or shaka? Seems like supporting dash if its possible might be a good thing.

zoriya · 2024-12-04T08:41:55Z

I personally didn't need more than hls.js, for rnv we would indeed need to consider alternatives before.